-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issue a cause of "Could not find step name" (2.6 bug). #11773
Conversation
This was happening because the code to generate step names was not excluding units that would not participate in combat, resulting in infrastructure units getting their own steps (which later did not match what the engine generated once the filtering took place). Uses the same logic as what's done for the battle to exclude units. Fixes: triplea-game#10647
...va/games/strategy/triplea/delegate/battle/steps/fire/general/FiringGroupSplitterGeneral.java
Show resolved
Hide resolved
Will these changes keep the ability of infrastructures to participate in combat beside being unable to take hits after the opening combat phase? For example, will an aircraft infrastructure (an aircraft unit which is also an infrastructure) still risk to be shot down by AA fire when attacking a territory defended by an AA Gun? For example, will an infrastructure with a positive offence or defence value still be able respectively to attack or to defend so long as at least one unit remaining in battle on its side is not an infrastructure? Same thing if the infrastructure is just providing (positive or negative) support (either normal or AA). These are abilities which are irrelevant for all basic games but not in TripleA games in general. |
Also, sometimes infrastructure units which are not participating in combat at all need to remain in combat if only to be able to retreat with the rest. For example, the horse units of Feudal Japan. |
@Cernelius I believe so, since I'm using the logic that was already being used by the battle code for this which does look at whether some infra units should participate. But I wouldn't mind if you can test these with the prerelease to make sure. |
Change Summary & Additional Notes
This was happening because the code to generate step names was not excluding units that would not participate in combat, resulting in infrastructure units getting their own steps (which later did not match what the engine generated once the filtering took place).
Uses the same logic as what's done for the battle to exclude units.
This change required adjusting a bunch of tests that were previously not careful about which had mistakes in setting up mock battles where the units didn't match the territory (in terms of sea vs. land). Also makes the tests to mock game data properties leniently, so that only the ones being set to true need to be specified (removing lots of LOC).
Includes a unit test.
Fixes: #10647
Note: Doesn't fix #11617, as the root cause of that error is different, despite the actual error being the same. I will fix that in a separate PR.
Release Note